fix(sandboxes): bound background output retrieval - #862
fix(sandboxes): bound background output retrieval#862DamianB-BitFlipper wants to merge 3 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ca296c473
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…tput-burst # Conflicts: # packages/prime-sandboxes/tests/test_batch_status.py
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 822778c. Configure here.
| await asyncio.shield(completion) | ||
| except asyncio.CancelledError: | ||
| pass | ||
|
|
There was a problem hiding this comment.
Cancelled fetch poisons later waiters
High Severity
When the last async waiter times out or is cancelled, _detach_waiter aborts the in-flight fetch_task but leaves that operation in _inflight until cleanup finishes. A concurrent get() can join the doomed operation and then raise operation.error, so an innocent caller receives CancelledError instead of output or a retry.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 822778c. Configure here.
| self._round_robin.append(job.sandbox_id) | ||
| queue.append(operation) | ||
| self._pending_count += 1 | ||
| self._promote_locked() |
There was a problem hiding this comment.
Queue wait skips inflight dedup
Medium Severity
After waiting for _pending_count to drop below queue_size, both coordinators create a new operation and write _inflight[key] without checking whether another waiter already inserted that key. Two callers for the same job can overwrite each other, break dedup, and run duplicate output reads.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 822778c. Configure here.
1 participant
Caps background-job output reads at 20 concurrent requests, retrieves stdout/stderr
sequentially, preserves completed exit codes when output retrieval fails, adds jittered retries and
errno diagnostics, and includes a 100-job concurrency test.
Verification:
Note
Medium Risk
Changes core sandbox client polling, concurrency, and deletion ordering for VM background jobs; behavior is more resilient but callers may see new error fields and different timing under load.
Overview
Splits VM background-job polling from stdout/stderr hydration so latency-sensitive callers can use
get_background_job_status/get_background_job_statuses(and batch coalescing) without triggering output downloads; full tails still come fromget_background_job/get_background_jobs.Adds client-wide output coordinators (sync and async) that cap concurrent output work (defaults: 20 active, 200 queued, 64 MiB LRU cache), deduplicate in-flight reads, schedule sandboxes round-robin, and fetch stdout then stderr under a bounded deadline.
BackgroundJobStatusnow includesstdout_error/stderr_error; when an exit code is known,completedstays true even if a stream could not be retrieved.Tightens lifecycle around deletion and close: sandbox-scoped leases gain admission checks; delete waits for active work and cancels queued output; async close joins output workers before transports. Read-file retries use jittered backoff and richer diagnostics (nested
errno) in API errors.Reviewed by Cursor Bugbot for commit 822778c. Bugbot is set up for automated code reviews on this repo. Configure here.